Skip to content

Conversation

@igiguere
Copy link
Contributor

@igiguere igiguere commented Jan 16, 2026

https://issues.apache.org/jira/browse/SOLR-17436

Description

Create a JAX-RS V2 equivalent for /admin/metrics.

Solution

Add interface MetricsApi, and implementation GetMetrics, as an extension of JerseyResource (via AdminAPIBase).

The new full URL path is /api/metrics. The metrics response is a StreamingOutput.

Add class MetricsUtil to re-group logic used by both MetricsHandler and GetMetrics. Both classes get their information directly from SolrMetricManager.

Adjust documentation.

Tests

Added unit tests for the GetMetrics implementation of MetricsApi.

Added unit tests for SolrJ MetricsRequest.

Functional tests on "devSlim", with a mini cluster of 2 nodes.

Checklist

Please review the following and check all that apply:

  • I have reviewed the guidelines for How to Contribute and my code conforms to the standards described there to the best of my ability.
  • I have created a Jira issue and added the issue ID to my pull request title.
  • I have given Solr maintainers access to contribute to my PR branch. (optional but recommended, not available for branches on forks living under an organisation)
  • I have developed this patch against the main branch.
  • I have run ./gradlew check.
  • I have added tests for my changes.
  • I have added documentation for the Reference Guide
  • I have added a changelog entry for my change

Isabelle Giguere and others added 5 commits January 15, 2026 10:00
Add interface MetricsApi and class GetMetrics.  Add MetricsUtil to merge
the List<MetricSnapshot> in either MetricsHandler or GetMetrics.

Implementations in MessageBodyWriters: not sure if needed.

Note in SplitShardCmd: this appears to have been broken before.  The
Solr response might always be null now.

More testing and cleaning-up to do.
Fix compilation and add unit tests
Adjust documentation.  Set ResponseParser in MetricsRequest constructor.
Clean-up.
Last clean-up after testing proxyToNodes() on a local cluster.
Isabelle Giguere added 2 commits January 16, 2026 12:51
add changelog, and commit result of tidy
@igiguere igiguere marked this pull request as ready for review January 16, 2026 21:49
Copy link
Contributor

@gerlowskija gerlowskija left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Was very excited to see this PR @igiguere , thanks for putting this together!

I've only reviewed about half the PR but wanted to publish what I have so far.

Everything looks great overall so far, though there were a few small things I think will need tweaked that I've tried to call out inline. Lmk if any of those comments are unclear or you disagree and I'll try to respond when I come back for the second half later!

Copy link
Contributor

@gerlowskija gerlowskija left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Alright, finished my review. Left a few more in-line comments, though things look very good overall. Just a few smaller things I think we'll need to change.

Let me know if you have any questions or I can help with anything!

=== OpenMetrics

OpenMetrics format is available from the `/admin/metrics` endpoint by providing the `wt=openmetrics` parameter or by passing the Accept header `application/openmetrics-text;version=1.0.0`. OpenMetrics is an extension of the Prometheus format that adds additional metadata and exemplars.
OpenMetrics format is available from the `/metrics` endpoint by providing the `wt=openmetrics` parameter or by passing the Accept header `application/openmetrics-text;version=1.0.0`. OpenMetrics is an extension of the Prometheus format that adds additional metadata and exemplars.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[Q] Huh - I'm surprised that these docs mention the "Accept" header even before you added a v2 endpoint in this PR. I didn't think anything on the v1 side looked at "Accept". I guess that's a good thing, I'm just surprised is all...

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I had written something custom to check content-type but just for OpenMetrics but not prometheus. See my comment above.


/** Request to "/admin/metrics" */
public class MetricsRequest extends SolrRequest<SolrResponse> {
public class MetricsRequest extends SolrRequest<InputStreamResponse> {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[Q] Why change the SolrJ class that corresponds to the v1 metrics API? Your PR only modifies the v2 API, right?

Copy link
Contributor Author

@igiguere igiguere Jan 20, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

V1, since Solr 10, returns a stream (prometheus or openmetrics), not a SolrResponse (where I would expect a NamedList... right?).
If I recall, the only place in Solr code where MetricsRequest is used is a method somewhere in the CLI tools that stars with something like : if (true) return; The method is disabled, and there's a couple of comments about tickets to be done.
I think the type param InputStreamResponse makes it clearer that this is not a typical Solr response.

Copy link
Contributor

@mlbiscoc mlbiscoc left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks @igiguere for doing this! The timing was great as the metrics api just had a huge overhaul in 10 so getting up to date on V2 gets us closer to a more modern path.

=== OpenMetrics

OpenMetrics format is available from the `/admin/metrics` endpoint by providing the `wt=openmetrics` parameter or by passing the Accept header `application/openmetrics-text;version=1.0.0`. OpenMetrics is an extension of the Prometheus format that adds additional metadata and exemplars.
OpenMetrics format is available from the `/metrics` endpoint by providing the `wt=openmetrics` parameter or by passing the Accept header `application/openmetrics-text;version=1.0.0`. OpenMetrics is an extension of the Prometheus format that adds additional metadata and exemplars.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I had written something custom to check content-type but just for OpenMetrics but not prometheus. See my comment above.

Address review comments.  Add query params to the API method. Fix
implementation and tests.

Adjust documentation and comments.

Remove class MetricsResponse.

TODO : AdminHandlersProxy only supports V1
@igiguere
Copy link
Contributor Author

@gerlowskija, @mlbiscoc
I think I addressed most review comments, or commented here.
One remaining issue : AdminHandlersProxy only support V1. If we should still suport the "node" (or "nodes") param, there'S a bit more work there. I'll sleep on it...

gradlew clean check in progress, locally.

Isabelle Giguere and others added 6 commits January 23, 2026 14:41
Adapt AdminHandlersProxy to handle V2 API.

Change GetMetricsTest to send REST GET requests.

Adapt MetricsRequest to support V1 and V2, and set the response parser,
according to 'wt'... Should handle "Accept" header, but SolrRequest is
not aware of HTTP headers.  Adapt re-existing code (tests) accordingly.

Add unit tests for MetricsRequest
remove temp file, remove ingore
Address review comments: Keep just one utility class for metrics
(MetricUtils).  Remove debug "System.out" form unit tests.
@igiguere
Copy link
Contributor Author

@gerlowskija, @mlbiscoc I think I addressed most review comments, or commented here. One remaining issue : AdminHandlersProxy only support V1. If we should still suport the "node" (or "nodes") param, there'S a bit more work there. I'll sleep on it...

gradlew clean check in progress, locally.

AdminHandlersProxy now supports both V1 and V2.
gradlew clean check passed locally.

@igiguere igiguere requested a review from gerlowskija January 26, 2026 20:39
Remove unnecessary logging.  Remove debug System.out
Merge remote-tracking branch 'origin-solr/main' into
SOLR-17436-V2-Metrics-API
@igiguere igiguere requested a review from mlbiscoc January 27, 2026 23:31
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants